Skip to content

Fix broken provide rule #50

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 22, 2020
Merged

Fix broken provide rule #50

merged 1 commit into from
Oct 22, 2020

Conversation

stof
Copy link
Contributor

@stof stof commented Oct 21, 2020

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets see composer/composer#9308
Documentation n/a
License MIT

What's in this PR?

This package provides the client implementation. But it does not provide the interfaces from psr/http-client

Why?

Which problem does the PR fix? (remove this section if you linked an issue above)

Example Usage

// If you added new features, show examples of how to use them here
// (remove this section if not a new feature)

$foo = new Foo();

// Now we can do
$foo->doSomething();

Checklist

  • Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
  • Documentation pull request created (if not simply a bugfix)

To Do

  • If the PR is not complete but you want to discuss the approach, list what remains to be done here

andrew-demb
andrew-demb reviewed Oct 21, 2020
View reviewed changes
composer.json
@@ -22,8 +22,7 @@
"php-http/client-common": "^2.0"
},
"provide": {
"php-http/client-implementation": "1.0",
"psr/http-client": "1.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we replace it in favor psr/http-client-implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand your comment. this package does not provide and does not replace psr/http-client, which is the package providing the PSR ClientInterface. It even requires psr/http-client (which is probably why things work in Composer 1).

And my Pr has not change the fact that this package provides php-http/client-implementation (the line is edited only to remove the comma, as JSON does not support trailing commas in objects)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, that this package should provide implementations for 2 packages:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, indeed, I missed that the existing line was for php-http/client and not psr/http-client

@stof
Fix broken provide rule
cc3396b 
This package provides the client implementation. But it does not provide the interfaces from psr/http-client
@stof stof changed the title Remove broken provide rule Fix broken provide rule Oct 21, 2020
@stof
Copy link
Contributor Author

stof commented Oct 21, 2020

This is now fixed

@Seldaek Seldaek mentioned this pull request Oct 22, 2020
Closed
Nyholm
Nyholm approved these changes Oct 22, 2020
View reviewed changes
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

@Nyholm Nyholm merged commit feb7b0b into php-http:master Oct 22, 2020
@stof stof deleted the patch-1 branch October 22, 2020 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrew-demb andrew-demb andrew-demb left review comments

@Seldaek Seldaek Seldaek approved these changes

@Nyholm Nyholm Nyholm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stof @Seldaek @Nyholm @andrew-demb